-
-
Notifications
You must be signed in to change notification settings - Fork 36
(EARLY REVIEW/SPECULATIVE) Refactoring syntax spec #429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Before starting to review this, I would like us to establish clearly whether we are continuing to apply semantic line breaks to our markdown content, or follow some other line wrapping practice. My preference would be to continue with semantic line breaks, as it seems that we persist in revisiting the text, but I'd really rather not spend more time adjusting line breaks in PRs, so would be fine with an alternative if it reaches consensus among others. |
We're doing semantic line breaks. What I'm looking for here with the early review is a sense of whether this change is something to chase after at all. If this is worth pursuing, I'll go back and button up the text before you (and the rest of the WG) review it seriously at the detail level. Most of the text here was stolen from the pre-formatting-fix text, although some I stole from the on-going PR. Is this better or should I focus on the other PR? |
We'll get those merged first, since we've already expended effort to make the text and syntax there correct. However... I'm looking for a reaction to a proposal: do you find this structure better than the current one? If not or if it's not significantly better, I won't bother with it. |
Having read this (much easier here than in this PR), I don't think I prefer this to what we have now. I think our current text does a better job of leading the reader towards an understanding of MF2 syntax by first walking them through example messages before getting into the nitty-gritty syntactical details. For a specific example of what I mean, in the text proposed in this PR, the reader is first presented with the ABNF for a local variable declaration before variable references are even mentioned, and that latter definition is effectively left out except for what very specifically matters for its definition as a possible operand value. In our current text, variable references are the first thing we talk about after the |
I actually like this a lot, thank you @aphillips! Back when I originally wrote my spec proposal (#218) I struggled with the same repetition that you noticed. I wanted the Overview & Examples section to come first, to showcase the proposal before specifying it, but it was difficult to explain the examples without defining the productions and the concepts. So I ended up with repeating some parts which didn't feel very "spec-like" to me, and which carried over to the spec on To address @eemeli's concern, I still think it would be good to have some kind of Overview before the Messages and Syntax section. It doesn't have to be as extensive as it is today; in fact, it shouldn't try to showcase every possible feature of the syntax — I think we should expect other documents to serve that purpose (e.g. tutorials, getting-started articles, etc.). But I'd like to have a few examples up front to help visualize what MesageFormat is and what problem space it exists in. Perhaps something similar to the examples in our current README? I can try to come up with something over the weekend. |
> | ||
> In general (and except where required by the syntax), whitespace carries no meaning | ||
> in the structure of a _message_. While many of the examples in this spec are written | ||
> on multiple lines, the formatting shown in this spec is primarily for readability. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This note is an example of what I'd put in the Overview section.
|
||
A complex message with 2 _selectors_ and 3 local variable _declarations_: | ||
> Example. In a Java `.properties` file, the message `hello` has exactly three spaces before and after |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are 2 spaces on both sides of Hello
in the example :)
spec/syntax.md
Outdated
|
||
Furthermore, a well-formed message is considered _valid_ | ||
if it meets additional semantic requirements about its structure, defined below. | ||
A _selector_ consists of a _match_ statement followed by at least one _variant_. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've always thought about this the other way around: expressions can either be placeholders or selectors, and the match
statement is where you put multiple selectors into.
Also, I'd consider variants to be part of the match
statement, rather than follow after it.
So in my mind it's (pseudo-ABNF):
match-stmt = 'match' 1*selector 1*variant
rather than:
selector = 'match' 1*expression 1*variant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like your reformulation very much. It solves a lot of circular jargon problems surrounding what the expression
s in selectors
are.
@stasm Thanks! Note that README.md is already a part of the spec and I think demonstrates some of the cases. We could put more there. |
Co-authored-by: Stanisław Małolepszy <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commenting mostly to clear this from my review queue.
As discussed on this week's call, I think a prerequisite for the changes proposed here is moving the intro-to-MF2 parts to the README or to some other overview doc. I would also find it easier if the structural changes to the prose and the ABNF changes were not mixed into one set of changes, but I understand that separating them might not be possible.
I am closing this PR in favor of the new one. @eemeli I did not include the ABNF changes in that PR. I will submit those in an independent PR. You can see my intentions in the prose. I also did not include changes outside syntax.md for now. |
In my effort to make the formatting correct, I found that the spec was repetitious and somewhat difficult to read. Here is a prototype refactor for early review.
This makes some normative changes, particularly it splits up
selectors
into named parts.Some sections could stand to be sub-sections. Have a look at it (preview it in the branch
aphillips-syntax-refactor
if that helps) and see if this is worth bringing to the larger group.Note: the TOC has not been updated, so ignore it.